Skip to content

[Resource Sharing] Allow multiple sharable resource types in single resource index #5713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 54 commits into from
Oct 21, 2025
Merged

[Resource Sharing] Allow multiple sharable resource types in single resource index #5713

merged 54 commits into from
Oct 21, 2025

Conversation

@cwperks
Copy link
Member

@cwperks cwperks commented Oct 14, 2025

Description

This PR contains changes to the ResourceAccessControlClient to start to refer to resourceType everywhere instead of resourceIndex. Currently, the repo assumes a 1-to-1 relationship between resource type and index, but this should be relaxed to allow multiple resource types in a single index (think dashboards saved objects).

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Enhancement

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

cwperks and others added 30 commits September 25, 2025 22:16
@cwperks
WIP on multiple sharable resource types per index
8575576 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Fix compilation issues
1663969 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Make more generic collection
c46cd2d 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Fix unit tests
6e4b679 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Make DocRequests specify type
f37c54c 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Few test fixes
14869cf 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Fix Share API tests
9a66864 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Fix tests
4c83aa5 
Signed-off-by: Craig Perkins <[email protected]>
@DarshitChanpura
Makes resources settings dynamically updateable
ebfd5bd 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Allows plugin to be control codepath based on protected resource types
32f903b 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Fix setting registration
5f7beb4 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Merge remote-tracking branch 'upstream/main' into dynamic-resource-se…
696482e 
…ttings
@DarshitChanpura
Adds changelog entry
c2988ea 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Corrects usage of resource-sharing flag
fbdae9f 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Fix share action rest channel consumer
a3493a6 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Fix sample plugin tests
9dd02a1 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Merge remote-tracking branch 'upstream/main' into dynamic-resource-se…
9930ae3 
…ttings
@DarshitChanpura
Merge remote-tracking branch 'upstream/main' into dynamic-resource-se…
1a94f40 
…ttings
@DarshitChanpura
Merge remote-tracking branch 'upstream/main' into dynamic-resource-se…
42b7230 
…ttings
@DarshitChanpura
Registers extensions regardless of whether their resources are marked…
b333d12 
… as protected and fixes share api action to throw 400 on resources not marked as protected

Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Adds tests for the new dynamic settings and update existing tests to …
ea333b9 
…consume new changes

Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Adds documentation
7c2c896 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Merge branch 'main' into dynamic-resource-settings
258229d 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Merge remote-tracking branch 'upstream/main' into dynamic-resource-se…
13204e2 
…ttings
@DarshitChanpura
Merge remote-tracking branch 'upstream/main' into dynamic-resource-se…
09b16eb 
…ttings
@DarshitChanpura
Fixes resource search request evaluation
8d186fe 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Adds caching to resource indices and makes read-write operations thre…
b707959 
…ad-safe in resource-plugin-info class

Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Fixes type enabled check
877bb11 
Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Refactors resource type fetch logic
f9fb61a 
Signed-off-by: Darshit Chanpura <[email protected]>
@cwperks
Rebase with main
0bd05b9 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Clean up changes
7df24dc 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Use correct method
4dc058a 
Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Copy link
Member Author

cwperks commented Oct 14, 2025

I plan to add new integration tests...either in this PR or another that add a second resource type (sample-resource-group) and keep records in the same index as sample-resource and then run through different scenarios for sharing.

@codecov
Copy link

codecov bot commented Oct 16, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 61.71171% with 170 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.89%. Comparing base (2dcde2e) to head (4b3d8da).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ava/org/opensearch/sample/SampleResourceGroup.java 0.00% 28 Missing ⚠️
...i/migrate/MigrateResourceSharingInfoApiAction.java 56.41% 11 Missing and 6 partials ⚠️
...ons/transport/GetResourceGroupTransportAction.java 48.38% 14 Missing and 2 partials ⚠️
.../transport/CreateResourceGroupTransportAction.java 75.00% 11 Missing and 3 partials ⚠️
...arch/security/resources/ResourceAccessHandler.java 56.00% 8 Missing and 3 partials ⚠️
.../transport/DeleteResourceGroupTransportAction.java 63.63% 6 Missing and 2 partials ⚠️
...ctions/rest/create/CreateResourceGroupRequest.java 46.15% 7 Missing ⚠️
...ctions/rest/create/UpdateResourceGroupRequest.java 58.82% 7 Missing ⚠️
...ons/rest/search/SearchResourceGroupRestAction.java 30.00% 7 Missing ⚠️
.../transport/UpdateResourceGroupTransportAction.java 77.77% 5 Missing and 1 partial ⚠️
... and 14 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5713      +/-   ##
==========================================
- Coverage   73.14%   72.89%   -0.26%     
==========================================
  Files         412      435      +23     
  Lines       26167    26548     +381     
  Branches     3963     3979      +16     
==========================================
+ Hits        19141    19351     +210     
- Misses       5139     5286     +147     
- Partials     1887     1911      +24
Files with missing lines Coverage Δ
...pensearch/sample/SampleResourceGroupExtension.java 100.00% <100.00%> (ø)
...va/org/opensearch/sample/SampleResourcePlugin.java 96.87% <100.00%> (+1.22%) ⬆️
...actions/rest/create/CreateResourceGroupAction.java 100.00% <100.00%> (ø)
...actions/rest/create/UpdateResourceGroupAction.java 100.00% <100.00%> (ø)
...actions/rest/delete/DeleteResourceGroupAction.java 100.00% <100.00%> (ø)
...group/actions/rest/get/GetResourceGroupAction.java 100.00% <100.00%> (ø)
...p/actions/rest/get/GetResourceGroupRestAction.java 100.00% <100.00%> (ø)
...actions/rest/search/SearchResourceGroupAction.java 100.00% <100.00%> (ø)
...in/java/org/opensearch/sample/utils/Constants.java 0.00% <ø> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java 85.39% <ø> (ø)
... and 29 more

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cwperks cwperks mentioned this pull request Oct 17, 2025
Merged
5 tasks
DarshitChanpura
DarshitChanpura reviewed Oct 20, 2025
View reviewed changes
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be good addition to this new framework and will setup a path to implement linux-style ACLs to allow hierarchical access authorization.

I have left a few comments. PR looks good mostly otherwise.

DarshitChanpura
DarshitChanpura reviewed Oct 20, 2025
View reviewed changes
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for addressing the comments. left a final few. Will approve once addressed.

This was referenced Oct 21, 2025
Open
Open
@cwperks cwperks merged commit aa8629e into opensearch-project:main Oct 21, 2025
66 of 68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DarshitChanpura DarshitChanpura DarshitChanpura approved these changes

@RyanL1997 RyanL1997 RyanL1997 approved these changes

@derek-ho derek-ho Awaiting requested review from derek-ho derek-ho is a code owner

@nibix nibix Awaiting requested review from nibix nibix is a code owner

@peternied peternied Awaiting requested review from peternied peternied is a code owner

@reta reta Awaiting requested review from reta reta is a code owner

@shikharj05 shikharj05 Awaiting requested review from shikharj05 shikharj05 is a code owner

@willyborankin willyborankin Awaiting requested review from willyborankin willyborankin is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cwperks @DarshitChanpura @RyanL1997